Skip to content

Conversation

nnethercote
Copy link
Contributor

I was looking closely at the arenas code and here are some small improvement to readability.

Because it's always `'tcx`. In fact, some of them use a mixture of
passed-in `$tcx` and hard-coded `'tcx`, so no other lifetime would even
work.

This makes the code easier to read.
@rust-highfive
Copy link
Contributor

r? @oli-obk

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 17, 2021
@nnethercote
Copy link
Contributor Author

@bors rollup=always

Because this is a combination of comment changes and trivial code changes.

@oli-obk
Copy link
Contributor

oli-obk commented Nov 18, 2021

@bors r+

@bors
Copy link
Collaborator

bors commented Nov 18, 2021

📌 Commit e73784d has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 18, 2021
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Nov 18, 2021
Arenas cleanup

I was looking closely at the arenas code and here are some small improvement to readability.
@apiraino apiraino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Nov 18, 2021
@JohnTitor
Copy link
Member

Failed in rollup: #91016 (comment)
@bors r-

 error: this URL is not a hyperlink
   --> compiler/rustc_arena/src/lib.rs:338:13
    |
338 |     /// see fitzgeraldnick.com/2019/11/01/always-bump-downwards.html.)
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `<fitzgeraldnick.com/2019/11/01/always-bump-downwards.html.>`
    |
    = note: `-D rustdoc::bare-urls` implied by `-D warnings`
    = note: bare URLs are not automatically turned into clickable links

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 18, 2021
Also use `Default::default()` in one `TypedArena::default()`, for
consistency with `DroplessArena::default()`.
@nnethercote
Copy link
Contributor Author

 error: this URL is not a hyperlink
   --> compiler/rustc_arena/src/lib.rs:338:13
    |
338 |     /// see fitzgeraldnick.com/2019/11/01/always-bump-downwards.html.)
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `<fitzgeraldnick.com/2019/11/01/always-bump-downwards.html.>`

Huh, I didn't see this with ./x.py test. Is there a command I could have run locally to catch this?

Also, curious that the leading https:// has been stripped in that error message.

Anyway, I've updated: @bors r=oli-obk

@bors
Copy link
Collaborator

bors commented Nov 18, 2021

📌 Commit 0a89598 has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 18, 2021
@JohnTitor
Copy link
Member

Huh, I didn't see this with ./x.py test. Is there a command I could have run locally to catch this?

The warning is generated by rustdoc so you could see it via ./x.py doc, I guess (yeah, it's kinda annoying to use that command just for a compiler/library change..., I don't know how we trigger it via other commands.)

Also, curious that the leading https:// has been stripped in that error message.

The scheme somehow has been stripped when copy/pasting 🤔, it's not a fault of the warning.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 18, 2021
Arenas cleanup

I was looking closely at the arenas code and here are some small improvement to readability.
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 19, 2021
Rollup of 8 pull requests

Successful merges:

 - rust-lang#89258 (Make char conversion functions unstably const)
 - rust-lang#90578 (add const generics test)
 - rust-lang#90633 (Refactor single variant `Candidate` enum into a struct)
 - rust-lang#90800 (bootstap: create .cargo/config only if not present)
 - rust-lang#90942 (windows: Return the "Not Found" error when a path is empty)
 - rust-lang#90947 (Move some tests to more reasonable directories - 9.5)
 - rust-lang#90961 (Suggest removal of arguments for unit variant, not replacement)
 - rust-lang#90990 (Arenas cleanup)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 1576a7c into rust-lang:master Nov 19, 2021
@rustbot rustbot added this to the 1.58.0 milestone Nov 19, 2021
@nnethercote nnethercote deleted the arenas-cleanup branch November 19, 2021 09:40
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 27, 2022
…oli-obk

More arena cleanups

A sequel to rust-lang#90990.

r? `@oli-obk`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants